Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CommandPost: transparently allow multi-word commands #224

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rjbs
Copy link
Member

@rjbs rjbs commented Feb 13, 2024

A bunch of the lingering EasyListening reactors are doing that because they want commands like "verb subthing" and "verb othersubthing". The CommandPost translations of that would come in two forms, until now, neither very appealing.

For one, you might make a "verb" command which then subparses. This loses a bunch of nice properties of commandpost, like the parser system.

For the other, you could make a responder, which just has the same problem.

I've long considered a means to say "add this reactor with a prefix word", so you could make "subthing" and "othersubthing" commands, and give the reactor a "verb" prefix. That would also allow the reactor to be configured twice with distinct prefixes. I think this is still a good idea.

The change made in this commit, though, is smaller in scope. It allows command names and aliases to have spaces in them. If they do, they're matched differently. We can't just split off the first word and look things up in a hash. Instead, we do a regex match. To avoid doing 100 regex matches per message if there are 100 commands, we only do the regex match for multi-word commands, and only if any multi-word commands have been registered. This implementation detail should be invisible to the user, and so can be changed later.

Prefix conflicts (the command "eat" and "eat pie" both being registered, for example) are not handled. They will handled implicitly by the fact that the PotentialReactions of commands are always flagged exclusive.

A bunch of the lingering EasyListening reactors are doing that because
they want commands like "verb subthing" and "verb othersubthing".  The
CommandPost translations of that would come in two forms, until now,
neither very appealing.

For one, you might make a "verb" command which then subparses.  This
loses a bunch of nice properties of commandpost, like the parser system.

For the other, you could make a responder, which just has the same
problem.

I've long considered a means to say "add this reactor with a prefix
word", so you could make "subthing" and "othersubthing" commands, and
give the reactor a "verb" prefix.  That would also allow the reactor to
be configured twice with distinct prefixes.  I think this is still a
good idea.

The change made in this commit, though, is smaller in scope.  It allows
command names and aliases to have spaces in them.  If they do, they're
matched differently.  We can't just split off the first word and look
things up in a hash.  Instead, we do a regex match.  To avoid doing 100
regex matches per message if there are 100 commands, we *only* do the
regex match for multi-word commands, and only if any multi-word commands
have been registered.  This implementation detail should be invisible to
the user, and so can be changed later.

Prefix conflicts (the command "eat" and "eat pie" both being registered,
for example) are not handled.  They will handled implicitly by the fact
that the PotentialReactions of commands are always flagged exclusive.
@rjbs rjbs force-pushed the main branch 4 times, most recently from aafd26b to eee1d40 Compare July 4, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant